-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hey @ptrendx , Thanks for submitting the PR
CI supported jobs: [sanity, clang, centos-gpu, centos-cpu, windows-gpu, miscellaneous, unix-cpu, windows-cpu, edge, website, unix-gpu] Note: |
@mxnet-bot run ci [centos-cpu, centos-gpu, unix-gpu] |
Jenkins CI successfully triggered : [centos-gpu, centos-cpu, unix-gpu] |
@mxnet-bot run ci [centos-cpu] |
Jenkins CI successfully triggered : [centos-cpu] |
@mxnet-bot run ci [centos-cpu] |
Jenkins CI successfully triggered : [centos-cpu] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an appropriate fix for this operator. LGTM.
Description
This commit fixes 2 problems:
uint8_t
, the maximum workspace size without setting large tensor support was 2GB, which was too small for some cases, without any error reporting (going over that limit would make the index_t in Shape1 overflow, resulting in negative value, which would later be cast to size_t in the allocator and MXNet would try to allocate a huge amount of memory, resulting in OoM). This PR changes the type of the allocation to double so that the size limit for workspace rises to 16 GB when large tensor support is not enabled.Checklist
Essentials